Claude finished @uipreliga's task in 1m 59s —— View job

Code Review In Progress

Review Tasks

• Read review guidelines from .github/code_review.md
• Read project conventions from CLAUDE.md
• Get full PR diff
• Review each changed file for context
• Perform cross-file consistency checks
• Analyze design decisions
• Check for missing elements
• Provide structured feedback

Starting review...

The evalboard change moves /trends and /watchlist into the OSS edition on the grounds that they "derive purely from run artifacts." That conflates two different bars: "runs without internal config" (portability — true) vs. "produces something meaningful for a generic adopter" (product fit — not really). Portability is cleared; product fit is the unaddressed concern, and it's the one that matters for an OSS-credibility play. Reasons these shouldn't ship OSS as-is:

• They assume an operating model OSS users won't have. The value prop — "last 10 runs," regression (recent-vs-prev pass rate), streaks, volatility sparklines, "never passed across the window" — assumes a recurring scheduled run of a stable suite (the UiPath nightly model). An adopter doing a handful of ad-hoc experiment runs gets degenerate output. Not hardcoded — assumed.
• Graceful ≠ useful. Several panels go blank without internal enrichment: dominantFailureTags needs a review index (readRunReviewIndex, not an OSS-core artifact), the maturity column needs matureSkipped (not produced by OSS core — always false in a bare clone), component-SHA attribution needs CI wiring. A new adopter lands on a half-empty Watchlist and reads it as broken — worse than not shipping it.

@akshaylive thanks — all three points taken.

1. Separate PR. Done. This PR is now the weight: 0 fix alone (plus the small doc/report/evalboard corrections); the retry work moved to #44, stacked on this branch. Rebased onto latest main while I was in there.

2. retry shouldn't live under run_limits. Agreed, and moved to a top-level retry: block in #44. Your framing is the right one: run_limits holds caps that abort a run, while a retry policy is how hard the run tries to survive a transient error. Worth noting run_limits had already drifted in this direction (count_cached_input, count_cache_creation, stop_early, expected_turns aren't caps either) — which I took as an argument for not adding a fifth exception. It's now a first-class 4th -D root next to agent/run_limits/sandbox, so -D retry.max_retries=0 merges through the same resolve_root engine at every config layer.

3. "Would this clear the working directory between retries?" No, it does not — good catch, and it's a real pre-existing gap rather than something the retry knob introduced:

• Retry wraps three seams in orchestrator.py: sandbox setup, agent start, and _communicate_with_retry.
• On a communicate retry the sandbox is untouched — files the crashed attempt already wrote survive into the retried attempt, and the same agent/SDK session is reused, so the re-sent prompt lands in a conversation that already contains the failed trajectory.
• The crashed partial TurnRecord is also drained onto the result (_on_attempt_failure), so trajectory-observing criteria — command_executed, commands_efficiency, skill_triggered — see both attempts. commands_efficiency in particular scores worse purely because a retry happened.

I chose to document this rather than add a reset seam: clearing the sandbox mid-task would discard work the agent had already done and budget already spent, and preserving partials is deliberate (crash forensics). #44 adds a retry section to TASK_DEFINITION_GUIDE.md spelling out the in-place semantics, with retry.max_retries: 0 as the escape hatch when you want a clean single-attempt trajectory. If you'd rather have an explicit reset option, I'm happy to add it — say the word and I'll do it in #44.

@bai-uipath you're right, and I've dropped the evalboard change from this PR entirely rather than argue it.

The justification in my PR description — "they derive purely from run artifacts" — was true and answered the wrong question. Portability was the bar I checked; product fit is the bar that matters here, and I didn't check it. Your two specifics hold up when I trace them:

• matureSkipped has no producer in src/. grep -rn mature_skipped src/ returns nothing — the Python core never emits mature_skipped, so lib/trends.ts falls through to ?? false on every task in a bare clone. The maturity column is structurally dead OSS-side, not just sparse.
• dominantFailureTags needs the review index. app/watchlist/page.tsx → loadRecentRuns (lib/overview.ts) → readRunReviewIndex (lib/reviews.ts), and review.json is produced by the coder-eval-review skill, not by core. A plain adopter gets an empty tag list.

So a new adopter lands on a Watchlist with a dead column and an empty failure-tag panel, and reasonably concludes the page is broken. That's worse than not shipping it, exactly as you said.

Dropping it also fixes a packaging problem @akshaylive flagged on the retry policy — this PR is now the weight: 0 fix alone, which is what it claims to be.

If we do want these OSS-side later, the prerequisite is core actually emitting the fields the panels read (mature_skipped at minimum, and some core-produced equivalent of the review index), which is its own piece of work rather than a nav-link change. Happy to open an issue for that if you think it's worth tracking.